-
Notifications
You must be signed in to change notification settings - Fork 312
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
rproxy: add incr/incrby/decr/decrby #146
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
有一个一堆rpc_code的地方,应该是proxy_layer.cpp下,把INC, CHECK_AND_SET这些code的allow inline都设置为1
@@ -938,6 +942,104 @@ void redis_parser::geo_radius_by_member(message_entry &entry) | |||
hash_key, "", radius_m, count, sort_type, 2000, search_callback); | |||
} | |||
|
|||
void redis_parser::incr(message_entry &entry) { counter_internal(entry); } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
補充一下注釋?像其他函數一樣。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
这里不用加注释,因为函数名本身就是自解释的。其他有的加了注释是因为跟redis原生命令有些出入
return; | ||
} | ||
} else { | ||
dassert(false, "command not support"); |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
@shengofsun INC已添加, CHECK_AND_SET目前rproxy还没使用,暂时不加吧 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@@ -35,6 +35,7 @@ proxy_stub::proxy_stub(const proxy_session::factory &f, | |||
dsn::task_spec::get(dsn::apps::RPC_RRDB_RRDB_GET_SCANNER_ACK)->allow_inline = true; | |||
dsn::task_spec::get(dsn::apps::RPC_RRDB_RRDB_SCAN_ACK)->allow_inline = true; | |||
dsn::task_spec::get(dsn::apps::RPC_RRDB_RRDB_CLEAR_SCANNER_ACK)->allow_inline = true; | |||
dsn::task_spec::get(dsn::apps::RPC_RRDB_RRDB_INCR)->allow_inline = true; |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
相应的单元测试有加吗?貌似没看到
这里主要是看到proxy这块之前也没有针对接口的单测,而那个redis协议的python脚本基本也不起作用(都想删掉的),所以现在都还没加,包括之前的redis geo的单测。 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
proxy.connection下的测试其实是对这些操作能work的一个基本测试。
anyway, 我先给你过了吧。后面你可以补一下这些新操作以及geo的测试。
* add incr/incrby/decr/decrby for redis proxy Former-commit-id: caee216d1b3c842d377b0c6086c3afa9165ed8a8 [formerly 2b92df0] Former-commit-id: a5b375e2b944e927eb9c771b935916c74328ac07
No description provided.